Skip to content

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Oct 14, 2022

We need to properly handle MapViewOfFileEx() failures; otherwise strange error messages might be reported in the following. E.g. bug72858.phpt is likely to fail on 32bit Windows with "Warning: shm_attach(): Failed for key 0x64: File exists".


Note that this issue also affects older PHP versions, but the chance of MapViewOfFileEx() failing is very small there, because we only tried to map an info segment there, instead of trying to map the whole segment now, due to #8648.

@Lewiscowles1986
Copy link
Contributor

This looks like a really nice fix. Can I ask if there is any desire to get this merged?

Looking into newer PHP build failures on Windows, it does seem that they could be influenced by seemingly small bugs like this fixes.

@krakjoe krakjoe changed the base branch from PHP-8.2 to master August 31, 2025 19:39
@krakjoe
Copy link
Member

krakjoe commented Aug 31, 2025

I'm unsure why no CI is running for this ? anyone ?

It looks correct and perfectly reasonable to merge, but I'm reluctant without running tests.

@cmb69 if you're around could you get this to run CI ?

@krakjoe krakjoe self-assigned this Aug 31, 2025
cmb69 added 2 commits August 31, 2025 22:44
We need to properly handle `MapViewOfFileEx()` failures; otherwise
strange error messages might be reported in the following.  E.g.
bug72858.phpt is likely to fail on 32bit Windows with "Warning:
shm_attach(): Failed for key 0x64: File exists".
@cmb69 cmb69 force-pushed the cmb/shmget-enomem branch from d0f6370 to b9a14f6 Compare August 31, 2025 20:44
@cmb69
Copy link
Member Author

cmb69 commented Aug 31, 2025

I have rebased onto current master, and now CI is running. Still not sure whether CI is really helpful here (as far as I know, we have only few tests regarding SHM on Windows). And I can barely remember what this PR is about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants